fix(grok): expose advertised reasoning levels - #6386
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
ApprovabilityVerdict: Needs human review This PR introduces new user-facing functionality - reasoning level selection for Grok models. The changes expose previously hidden metadata as UI controls and propagate new configuration through the ACP session. While thoroughly tested, this is a new capability from a new contributor to this area of the codebase. You can customize Macroscope's approvability policy. Learn more. |
534f90f to
e0fc12b
Compare
|
cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit e0fc12b. Configure here.
Intent: Keep Grok 4.6 usable from T3: plan-mode turns must stay reviewable over ACP, advertised reasoning levels and user-invocable skills must show in the composer, and the T3 runtime-mode control must actually drive Grok instead of ~/.grok/config.toml. Behavior: - Handle both x.ai/exit_plan_mode and _x.ai/exit_plan_mode, including wrapped payloads and null planContent. - Emit the existing proposed-plan card from planContent, or fall back to the last session plan.md body when Grok races the plan-file write. - Reply abandoned with a capture message so Grok unblocks without implementing in the same turn. - Detect enter_plan_mode and promote writes to ~/.grok/sessions/.../plan.md onto the same card. Ignore workspace plan.md files, dedupe identical markdown per turn, and clear fallback state on settle or a new non-steer turn. - Leave implement and request-changes to the existing proposed-plan follow-up; do not auto-approve plans even in full-access mode. - Map each Grok model's ACP _meta.reasoningEfforts into the existing Reasoning select, preserving labels and descriptions and exposing the applied effort as currentValue. Keep one default badge and do not invent a menu when metadata is absent. - Apply reasoning effort through session/set_model _meta.reasoningEffort, including effort-only changes. Do not carry an effort across model switches unless explicitly selected, skip set_model when nothing changes, and defer turn-time mutation until sendTurn validation succeeds. - Honor the selected effort in Grok-backed title, commit, PR, and branch-name generation. - Read user-invocable skills from grok inspect --json into the provider snapshot; use those names for both $ and /, skip bundled helpers, and leave both catalogs empty on inspect failure without failing provider discovery. - Supervised spawns `--permission-mode default` so the thread asks even if the Grok CLI config is always-approve or auto. - Auto-accept edits spawns `--permission-mode acceptEdits`. - Auto spawns `--permission-mode auto` and sends session/new (and session/load) `_meta.autoMode: true`. - Full access spawns `grok agent --always-approve stdio` and sends `_meta.yoloMode: true`. - Probe and text-generation ACP processes omit a T3 mode and keep `grok agent stdio`. - Always allow this session falls back to allow_once when Grok omits allow_always, then auto-approves later prompts in that session. - Auto still escalates risky calls to T3; it is not always-approve. Design constraints: - Keep Grok-specific ACP, inspect, and permission dialects at the adapter/provider boundary. Do not change contracts or other providers. - Reuse turn.proposed.completed, the existing plan card, and generic composer optionDescriptors / skills / slashCommands. Add no Grok-only approval prompt or new composer UI. - Do not spawn-bind --reasoning-effort or require a new thread to change effort; Grok applies _meta in place. - Do not map T3 Auto to yolo / bypassPermissions. That is Full access. - Do not restart mid-thread on a runtime-mode change (separate hang: upstream pingdotgg#6517). Integration: - Grok adapter, xAI ACP extension helpers, ACP mock, ACP session runtime (set_model and session/new|/load _meta), Grok spawn args, Grok model/provider snapshot, inspect catalog parser, text generation, focused tests, internals provider docs, install.md, and permission-modes.md. - Web, desktop, and mobile consume existing proposed-plan events, generic snapshot fields, and the existing runtime-mode picker with no client changes. Verification: - vp test run apps/server/src/provider/acp/GrokAcpSupport.test.ts apps/server/src/provider/Layers/GrokProvider.test.ts apps/server/src/provider/Drivers/GrokSkills.test.ts apps/server/src/provider/Layers/GrokAdapter.test.ts apps/server/src/provider/acp/XAiAcpExtension.test.ts - Runtime-mode slice: 57 passed (GrokAcpSupport, GrokAdapter, GrokProvider). - Earlier combined Grok slice: 64 passed including skills and xAI extension. Rebase notes: - Conflict hotspots are GrokAdapter plan handlers, sendTurn settlement, handleRequestPermission, buildGrokAcpSpawnInput, XAiAcpExtension.ts, GrokProvider discovered-model capabilities, GrokDriver inspect cwd, and AcpSessionRuntime setSessionModel plus session/new _meta. - Upstream pingdotgg#4514 (plan), pingdotgg#5403/pingdotgg#6386/pingdotgg#6887 (reasoning), pingdotgg#4109 (skills), pingdotgg#6502/pingdotgg#6626 (Always allow and spawn permission-mode). Drop this patch only when main handles both plan-exit spellings with live/fallback plan capture, maps ACP reasoningEfforts and effort-only set_model, publishes grok inspect skills to $ and /, forwards all four T3 modes onto Grok argv with autoMode/yoloMode on session setup, Supervised overrides config.toml, and Always allow does not cancel when allow_always is missing.
What Changed
session/set_modelmetadata, including same-model changes and explicit clearing back to the model defaultWhy
Grok 4.6 advertises Extra High, High, Medium, and Low reasoning levels through its ACP model metadata, but T3 discarded Grok model capabilities and only sent model IDs. That left the composer without a reasoning control and made same-model effort changes impossible.
This is a focused reasoning-only fix. It overlaps the reasoning portion of #6383, which appeared after this work started and also bundles Grok auth, rewind, and token-usage changes.
I also compared this implementation with the orchestrator-v2 work in #5160. This PR reuses the portable safeguards from that work: safe ACP token parsing, preserved descriptions, current-vs-default separation, one default badge, and symmetric effort clearing. It intentionally leaves out v2-only lifecycle scaffolding, spawn-bound locks, and fallback catalogs.
UI Changes
Before
Grok 4.6 had no reasoning control.
After
The composer reads Grok 4.6's live effort menu, and changing the selection updates the visible value.
The
Effortsuffix in this screenshot comes from Grok 4.6's ACP-provided labels. T3 displays only ACP-advertised options, preserving their labels verbatim without synthesizing a fallback menu.Verification
vp test run apps/server/src/provider/acp/GrokAcpSupport.test.ts apps/server/src/provider/Layers/GrokProvider.test.ts apps/server/src/provider/Layers/GrokAdapter.test.ts apps/server/src/textGeneration/GrokTextGeneration.test.ts(51 tests passed)T3_GROK_ACP_PROBE=1 vp test run apps/server/src/provider/acp/GrokAcpCliProbe.test.ts --reporter=verbose(4 tests passed against Grok CLI 1.0.3)vp run --filter=t3 typecheckvp fmt --checkandvp lintfor all changed filesChecklist
Implemented with gpt-5.6-sol through the Codex harness in T3 Code.
Note
Medium Risk
Changes Grok session model binding and
session/set_modelbehavior (including same-model effort and clearing), which affects live turns and auxiliary Grok prompts; risk is mitigated by validation, deferredset_modeluntil after turn validation, and broad tests.Overview
Grok reasoning effort is now wired end-to-end from ACP model metadata into the composer and runtime.
Discovery and UI contract:
buildGrokModelCapabilitiesturns each model’s_meta.reasoningEffort/reasoningEffortsinto areasoningEffortselect (labels, descriptions, default badge, token validation). Discovered Grok models use these capabilities instead of empty option lists.ACP dispatch:
setSessionModelaccepts optional_meta;applyGrokAcpModelSelectioncompares current vs requested effort and callssession/set_modelwith{ reasoningEffort }when the model or effort changes, or omits_metato clear effort on the same model. GrokAdapter trackscurrentReasoningEffort, applies selection after turn validation (so failed prep/validation does not callset_model), and avoids leaking prior-session effort on start when switching models. Grok text generation applies the same selection path.Tests and the mock ACP agent cover effort metadata; install docs mention the Reasoning control for supported Grok models.
Reviewed by Cursor Bugbot for commit e0fc12b. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Expose Grok reasoning effort levels from ACP model metadata in model capabilities
buildGrokModelCapabilitiesin GrokProvider.ts parses_meta.reasoningEffortand_meta.reasoningEffortsfrom ACP model metadata and returns areasoningEffortselect option when present; discovered models now use this instead of empty capabilities.applyGrokAcpModelSelectionin GrokAcpSupport.ts now tracks current and requested reasoning effort, triggeringsession/set_modelwith_meta.reasoningEffortwhen effort changes on the same model, and clearing it when omitted.AcpSessionRuntime.setSessionModelin AcpSessionRuntime.ts accepts an optional_metapayload so reasoning effort can be forwarded in set-model requests.currentReasoningEffortin session context and applies it alongside model selection onsendTurn;session/set_modelis no longer issued whensendTurnvalidation fails.Macroscope summarized e0fc12b.